fix(LogViewer): restore hover-driven chart cursor after extraction#14407
fix(LogViewer): restore hover-driven chart cursor after extraction#14407AhmWael wants to merge 2 commits into
Conversation
When LogViewerChart was extracted from LogViewerPage (mavlink#14354), the chart MouseArea lost hoverEnabled and the handler that updates the cursor while the pointer moves. The marker and value popup only updated on a short click-release, which regressed .bin chart inspection. Restore the pre-extraction interaction on desktop: - LogViewerChart: hoverEnabled, cursor update on press and on move (when not drag-zooming), hide marker on exit, refresh marker X when plotArea changes - LogViewerAltChart: same hover/press/move/exit behavior for the map altitude chart Mobile keeps click-only interaction (hoverEnabled disabled via ScreenTools).
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds hover-driven cursor updates and improves cursor/marker behavior in LogViewer charts (primarily on non-mobile), including hiding the marker when the pointer exits and keeping marker position in sync with plot-area changes.
Changes:
- Enable hover tracking on desktop (
hoverEnabled: !ScreenTools.isMobile) to update cursor position without clicking. - Update cursor position on press/move/release flows and hide marker on pointer exit.
- Refresh cursor pixel position when the chart plot area changes (main chart).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/AnalyzeView/LogViewer/LogViewerChart.qml | Adds hover support, updates cursor handling around zoom interactions, hides marker on exit, and refreshes marker position on plot-area changes. |
| src/AnalyzeView/LogViewer/LogViewerAltChart.qml | Adds hover support, updates cursor handling around zoom interactions, and clears marker on exit. |
| const dragWidth = _zoomSelectionRect.width | ||
| _zoomSelectionRect.visible = false | ||
| if (dragWidth < ScreenTools.defaultFontPixelWidth * 0.5) { | ||
| _updateCursorInfo(mouse.x, mouse.y, width, height) | ||
| return | ||
| } | ||
| const leftX = _pixelToAxisX(_zoomSelectionRect.x) | ||
| const rightX = _pixelToAxisX(_zoomSelectionRect.x + _zoomSelectionRect.width) | ||
| applyZoomRange(Math.min(leftX, rightX), Math.max(leftX, rightX)) | ||
| _updateCursorInfo(mouse.x, mouse.y, width, height) | ||
| } |
| const dragW = _zoomRect.width | ||
| _zoomRect.visible = false | ||
| if (dragW < ScreenTools.defaultFontPixelWidth * 0.5) { | ||
| root._updateCursor(mouse.x) | ||
| return | ||
| } | ||
| const leftX = root._pixelToAxisX(_zoomRect.x) | ||
| const rightX = root._pixelToAxisX(_zoomRect.x + _zoomRect.width) | ||
| root._applyZoom(Math.min(leftX, rightX), Math.max(leftX, rightX)) | ||
| root._updateCursor(mouse.x) | ||
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (26.57%) is below the target coverage (30.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #14407 +/- ##
==========================================
+ Coverage 25.47% 26.57% +1.10%
==========================================
Files 769 767 -2
Lines 65912 66285 +373
Branches 30495 30667 +172
==========================================
+ Hits 16788 17615 +827
+ Misses 37285 36159 -1126
- Partials 11839 12511 +672
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 4 passed, 45 failed, 7 skipped. Test Resultslinux-coverage: 88 passed, 0 skipped Code CoverageCoverage: 60.6% No baseline available for comparison Artifact Sizes
Updated: 2026-05-22 14:39:57 UTC • Triggered by: Android |
|
@DonLakeFlyer Can you take a look at this PR? |
|
I changed this because of mobile. I'd rather keep them the same but if you think this hover feature is critical for analysis I can live with the difference. Do you think it's important enough for feature set to differ? |
I find it really useful for log analysis on desktop. Being able to just hover through the trend and inspect values feels much more natural than having to click every point to update the readings. |
|
Thanks, makes total sense. Sorry I didn't take something like that into account. |
|
It should be ready for review now @DonLakeFlyer |
Description
When
LogViewerChartwas extracted fromLogViewerPagein #14354, the chartMouseArealosthoverEnabledand the logic that updates the cursor while the pointer moves. The vertical marker and value popup only updated on a short click-release, which regressed inspection of DataFlash (.bin) and ULog charts.This PR restores the pre-extraction desktop behavior:
hoverEnabledon desktop, cursor updates on press and while moving (when not drag-zooming), marker hidden when the pointer leaves the chart, marker X refreshed whenplotArealayout changesMobile remains click-only (
hoverEnableddisabled viaScreenTools.isMobile).Type of Change
Testing
Manual verification:
.bin(or.ulg) log.Platforms Tested
Flight Stacks Tested
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).